feat(context): add context manager#2547
Conversation
06171e3 to
a1337f0
Compare
a1337f0 to
a97206c
Compare
a97206c to
8f0feca
Compare
8f0feca to
3ef3431
Compare
3ef3431 to
a70ae1c
Compare
a70ae1c to
50e7824
Compare
| // Tool-pair partner protection: if adjacent message is protected and they form a pair | ||
| const msg = messages[index]! | ||
| const hasToolResult = msg.content.some((b) => b.type === 'toolResultBlock') | ||
| if (hasToolResult && index > 0 && index - 1 < protectFirst) return true |
There was a problem hiding this comment.
Issue: This does not verify that messages[index - 1] actually contains a matching toolUseBlock. It only checks that the current message has a toolResult and the previous message is within protectFirst. If message at index - 1 happens to be a regular text message at the boundary of protectFirst, this incorrectly marks the current message as protected.
Suggestion: Add a check that validates the previous message contains a toolUseBlock with a matching toolUseId:
if (hasToolResult && index > 0 && index - 1 < protectFirst) {
const prev = messages[index - 1]!
const resultIds = new Set(
msg.content.filter((b): b is ToolResultBlock => b.type === 'toolResultBlock').map((b) => b.toolUseId)
)
if (prev.content.some((b) => b.type === 'toolUseBlock' && resultIds.has((b as ToolUseBlock).toolUseId))) {
return true
}
}In practice, the LLM API ordering (toolUse always precedes toolResult) may prevent this from manifesting as a user-visible bug, but the validation keeps the function correct regardless of message arrangement.
| export type OffloaderConfig = { | ||
| /** Token threshold above which tool results are offloaded. Defaults to 2500. */ | ||
| threshold?: number | ||
| /** Number of tokens to keep as an inline preview. Defaults to 500. */ |
There was a problem hiding this comment.
Issue: previewTokens default is documented as 500 here (and repeated on line 46), but the actual fallback on line 159 is ?? 1500. The PR description's table also states 1500.
Suggestion: Update the TSDoc to say "Defaults to 1500" to match the implementation.
| if (hasToolResult && index > 0 && index - 1 < protectFirst) return true | ||
|
|
||
| const hasToolUse = msg.content.some((b) => b.type === 'toolUseBlock') | ||
| if (hasToolUse && index + 1 < messages.length && index + 1 < protectFirst) return true |
There was a problem hiding this comment.
Issue: This condition is unreachable. We only arrive here when index >= protectFirst (line 123 already returned true for index < protectFirst). For index + 1 < protectFirst to be true, we'd need index < protectFirst - 1, which contradicts index >= protectFirst.
Suggestion: Remove this dead branch or rewrite the tool-pair partner logic. If the intent is "protect a toolUse whose partner toolResult is in the protected range", note that toolResult always comes after toolUse in message ordering, so the toolUse is always at a lower index — meaning it would already be protected by line 123.
| continue | ||
| } | ||
|
|
||
| const hasToolUse = msg.content.some((b) => b.type === 'toolUseBlock') |
There was a problem hiding this comment.
Issue: The findValidTrimPoint function checks for toolUseBlock on user-role messages (lines 73-80), but toolUseBlocks only appear in assistant messages. Since line 63 already skips non-user messages, this branch is dead code.
The same pattern appears in adjustSplitForToolPairs in summarize.ts.
Suggestion: Remove the dead hasToolUse check or restructure the logic to correctly handle the trim boundary. The actual concern is: don't start the "kept" portion at a toolResult (user message) that is the result of a toolUse (assistant message) immediately before it — which is already handled by the toolResultBlock check on line 68.
| * ```typescript | ||
| * // Config shorthand (most users) | ||
| * const agent = new Agent({ contextManager: "auto" }) | ||
| * |
There was a problem hiding this comment.
Issue: The TSDoc @example (lines 82-88) shows passing a ContextManager class instance to Agent({ contextManager: cm }), but ContextManagerParam is typed as ContextStrategyValue | ContextManagerConfig — it doesn't accept a ContextManager instance. This example would fail type-checking.
Suggestion: Either update ContextManagerParam to also accept ContextManager instances (if that's the intended "power user" path), or fix the example to show the config-object approach:
const agent = new Agent({ contextManager: { storage: new S3Storage("bucket") } })| continue | ||
| } | ||
|
|
||
| const hasToolUse = msg.content.some((b) => b.type === 'toolUseBlock') |
There was a problem hiding this comment.
Issue: The adjustSplitForToolPairs function has the same dead code pattern as findValidTrimPoint in truncate.ts — checking for toolUseBlock on messages that have already passed the role !== 'user' skip (line 128-129 skips non-user messages, so the message at idx is always user-role, which never contains toolUseBlock).
Suggestion: Same as the comment on truncate.ts — consider removing the dead branch or documenting why it exists as defensive coding.
| ) | ||
| } | ||
| this._conversationManager = new NullConversationManager() | ||
| } else if (contextManagerPlugin) { |
There was a problem hiding this comment.
Issue: When a non-stateful model has both contextManager and conversationManager set, the conversationManager is silently ignored (line 365-366 takes priority). This could confuse users who set both accidentally.
Suggestion: Consider logging a warning when both are provided, e.g.:
} else if (contextManagerPlugin) {
if (config?.conversationManager) {
logger.warn('contextManager takes priority over conversationManager — conversationManager will be ignored')
}
this._conversationManager = new NullConversationManager()
}|
Assessment: Comment Well-architected feature with clear separation of concerns between the facade ( Review Categories
The overall design aligns well with SDK tenets — particularly composability and "provide both low-level and high-level APIs". |
29d1f4d to
686bdfb
Compare
| * A message is protected if it is pinned, within the protectFirst range, | ||
| * or is a tool-pair partner of a protected message. | ||
| */ | ||
| export function isProtected(messages: Message[], index: number, protectFirst?: number): boolean { |
There was a problem hiding this comment.
Maybe a more descriptive function name
There was a problem hiding this comment.
thoughts on isMessageEvictionProtected? isEvictionProtected?
| const SUMMARIZATION_PROMPT = `You are a conversation summarizer. Provide a concise summary of the conversation history. | ||
|
|
||
| Format Requirements: | ||
| - You MUST create a structured and concise summary in bullet-point format. |
There was a problem hiding this comment.
confirming bullet-point format
There was a problem hiding this comment.
ported over from summarizing conversation manager -- is that fine?
686bdfb to
8a6f0db
Compare
|
Closing out in favor of context facade |
Description
Implements the v1
contextManagerfacade as designed in strands-agents/docs#831.Adds a
contextManagerparameter toAgentConfigthat pre-composes the SDK's context management primitives into a single configuration surface. An internalContextManagerplugin composes sub-plugins (ContextCompression,ContextOffloader) that handle the actual behavior.Architecture
Sub-plugins work independently when used standalone. User-provided plugins with matching names take precedence over managed sub-plugins. When
contextManageris set,ContextCompressiontakes priority —NullConversationManageris used (same pattern as other dedicated-param plugins likeretryStrategy,sessionManager).What ships
contextManagerparameter onAgentConfig— accepts"auto"or a config objectContextCompressionplugin — proactive/reactive compression with own reduction logic (truncate or summarize)ContextOffloader— stays invended-plugins/context-offloader/, composed internally by ContextManagercontext-manager/compression/protection.ts) —pinMessageToolfor agent-controlled pinning at runtime. Internal utilities (pinMessage,unpinMessage,isPinned,isProtected) not exported; programmatic pinning API deferred.protectFirst— number of messages at the start of the conversation to protect from evictionestimateInputTokens()utility — shared token estimation insrc/context-manager/token-estimation.ts<summary>XML tags — summarized messages are wrapped in<summary>tags so the model can distinguish framework-injected summaries from user contentconversationManagermarked as pending deprecation — still works, JSDoc-taggedPublic API Surface
New on
AgentConfig:contextManager?: ContextManagerParamNew exports:
pinMessageTool(agent-invokable tool)ContextManagerParam(type)All classes (
ContextManager,ContextCompression,ContextOffloader) are internal.ContextOffloaderremains accessible via thevended-plugins/context-offloadersub-path for backward compat.Configuration model
Two semantics depending on whether
strategy: 'auto'is present:Override (strategy: 'auto') — starts with everything enabled, you override specific settings:
Additive (no strategy) — starts with nothing, you enable what you want:
Compression config (discriminated union on
method)Defaults (when enabled via "auto")
Plugin registration
contextManagermust be passed via the dedicated parameter — same pattern asconversationManager,retryStrategy, andsessionManager. No guards for misuse inplugins[](consistent with other special-cased plugins).Deprecation Plan
The following are marked as pending deprecation in v1 and will be removed in v2:
AgentConfig.conversationManager→contextManager: { compression: ... }Agent._estimateInputTokens()→ sharedestimateInputTokens()utilityBeforeModelCallEvent.projectedInputTokens→ futurecontextManagerbudget APIConversationManager,SlidingWindowConversationManager,SummarizingConversationManager,NullConversationManager→ContextCompressionpluginvended-plugins/context-offloader/sub-path → import from@strands-agents/sdkdirectlyBreaking Changes
None. All changes are additive. Existing behavior is unchanged when
contextManageris not set.Related Issues
Documentation PR
strands-agents/docs#831
Type of Change
New feature
Testing
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.